Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCDAV: map bad request and unimplemented codes #1354

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Dec 2, 2020

We now return a 400 bad request when a grpc call fails with an invalid argument status and a 501 not implemented when it fails with an unimplemented status. This prevents 500 errors when a user tries to add resources to the Share folder or a storage does not implement an action.

This is part of a series to get 500 errors returned as proper 400 errors: owncloud/product#7

@butonic butonic requested a review from labkode as a code owner December 2, 2020 08:16
@update-docs
Copy link

update-docs bot commented Dec 2, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@butonic butonic requested a review from ishank011 December 2, 2020 08:17
@butonic butonic force-pushed the ocdav-handle-bad-request branch from 01ec606 to e48e169 Compare December 2, 2020 09:17
@ishank011
Copy link
Contributor

@butonic can we make this a bit modular? So many switch cases make reading through the code a bit difficult.

@butonic butonic force-pushed the ocdav-handle-bad-request branch from e48e169 to f5f0b42 Compare December 2, 2020 16:23
@butonic
Copy link
Contributor Author

butonic commented Dec 2, 2020

@ishank011 I moved the switch to a function and now use that throughout the ocdav handler. Using subloggers this even allows logging additional fields, eg. src or dst. The handleErrorStatus function will render debug level logs, save for unknown status codes, which will be mapped to internal server error and be logged as error.

I deleted more lines than I added, so I'm happy.

ishank011
ishank011 previously approved these changes Dec 2, 2020
Copy link
Contributor

@ishank011 ishank011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Will merge once CI is green

@labkode
Copy link
Member

labkode commented Dec 3, 2020

@butonic maybe rebase

@butonic butonic force-pushed the ocdav-handle-bad-request branch from 74f541f to f374ded Compare December 3, 2020 07:14
@butonic butonic force-pushed the ocdav-handle-bad-request branch from f374ded to b8a6a42 Compare December 3, 2020 07:17
@butonic
Copy link
Contributor Author

butonic commented Dec 3, 2020

@labkode I had swallowed a conflic status when refactoring ... good to merge now.

@butonic butonic requested a review from ishank011 December 3, 2020 07:46
@labkode labkode merged commit 783e35c into cs3org:master Dec 3, 2020
@butonic butonic deleted the ocdav-handle-bad-request branch July 8, 2021 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants